feat: add secure Clipboard workspace with history and snippets - #335

Review summary

The security baseline here is solid, and I want to state what I verified so the remaining items are read in context: capture reads type names before payload bytes, so org.nspasteboard.ConcealedType producers are rejected before their data enters the process; AES-GCM with kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly and per-row AAD bound to the row UUID; crypto-shred covers both databases through one shared PluginPrivateDataKeychainIdentity; no network or shell surface anywhere in the plugin; export naming neutralizes path separators and leading dots. The IPC layer is also clean — length-prefixed framing validated before allocation, PropertyList + Codable rather than NSKeyedUnarchiver, timeout plus RLIMIT_AS and a resident-memory watchdog in the helper.

The items below are what I would like addressed. The first three are regressions against existing behavior with small diffs, not new work.

1. Regressions in existing behavior

1.1 canAssign lost its availability check

Sources/Core/Plugins/PluginHost.swift

Issue. The predicate went from two conditions to one, and the removed clause is the first line — this reads as a refactor slip rather than a decision. It affects every plugin that exposes action shortcuts: actions that are currently unavailable can now be bound.

// base
canAssign: availability.isAvailable
    && action.definition.capabilities.contains(.foregroundInteractive)

// this PR
canAssign: action.definition.capabilities.contains(.foregroundInteractive)

Fix. Restore availability.isAvailable &&. If allowing pre-binding of temporarily unavailable actions is intended, keep the change but add a test asserting that behavior and a changelog entry, so it does not remain in a form indistinguishable from an accidental deletion.

1.2 Selected-row contrast in the unified search palette

Sources/App/UnifiedSearchPaletteView.swift, Sources/MacToolsPluginKit/PluginPaletteComponents.swift

Issue. The selected-row background moved from Color.accentColor to Color(nsColor: .selectedContentBackgroundColor), but row content still uses the foreground styling designed for an accent-colored background (isSelected ? Color.white : Color.accentColor). Under a non-blue accent color or a light selection background, contrast is insufficient.

Fix. Pick one model rather than mixing them: either move row foregrounds to the semantic foreground that pairs with .selectedContentBackgroundColor, or restore the accent-color background.

1.3 insertNewlineIgnoringFieldEditor changed meaning

Sources/MacToolsPluginKit/PluginPaletteComponents.swift

Issue. This selector previously always resolved to "open the owning settings item". It is now folded into the same branch as plain Return, so without Cmd it activates the current result instead. This changes existing command-palette keyboard behavior and is unrelated to the clipboard feature.

Fix. Give the selector its own branch again and keep the original .openOwner result.

2. Data integrity

2.1 No whole-database integrity check in the incremental store

Plugins/ClipboardHistory/Sources/IncrementalEncryptedClipboardHistoryStore.swift

Issue. Per-row protection is good — GCM plus AAD bound to the row UUID means tampering with a row or relocating ciphertext between rows fails authentication. But load() treats an empty items table as a normal empty result, which is indistinguishable from "the user genuinely has no history". Replacing the database file with a valid empty one therefore wipes history without surfacing any error. The monolithic format this replaced had that protection implicitly, since the whole file was a single sealed box.

Fix. Add a single-row manifest table holding {rowCount, generation} sealed with the same key and a fixed AAD string:

CREATE TABLE IF NOT EXISTS store_manifest (
    id INTEGER PRIMARY KEY CHECK (id = 0),
    manifest BLOB NOT NULL
)

Writes are already wrapped in BEGIN IMMEDIATE TRANSACTION / COMMIT, so the manifest update folds into the same transaction. On load, open the manifest and verify rowCount matches the actual row count and that generation has not regressed; on mismatch, throw into the existing storageError path instead of returning an empty array. This needs a PRAGMA user_version bump, with the manifest initialized from the current row count during migration.

3. Main-thread work

These three conflict with the AGENTS.md rule that panel-state getters read existing snapshots and that long scans stay off the main thread. Items 3.2 and 3.3 reuse the same counters, so they are best done together.

3.1 primaryPanelState scans the full collection twice per call

Plugins/ClipboardHistory/Sources/ClipboardHistoryPlugin.swift, ClipboardHistoryController.swift

Issue. controller.historyItems and controller.savedItems are both items.filter(...). onStateChange fires on capture, OCR, and persistence events, so every host state rebuild pays two O(n) passes with n up to 10,000.

Fix. Maintain historyItemCount and savedItemCount in the controller, updated incrementally at the points where items changes (capture application, retention, mutation apply), and have the getter read the counters. A smaller alternative is invalidating a cached pair in items.didSet, which keeps the O(n) recompute but only on real mutations rather than on every getter call.

3.2 Full retention evaluation on every capture

Plugins/ClipboardHistory/Sources/ClipboardHistoryController.swift

Issue. ClipboardRetentionPolicy.evaluate filters, sorts, and compacts the whole candidate list synchronously on each successful capture, with n up to 10,000.

Fix. Most captures evict nothing. Add an O(1) precheck — history count below the cap, total bytes below the cap, oldest item not yet expired — and insert directly when all three hold, falling back to the current path only when eviction is possible. This uses the counters from 3.1 plus an oldest-captured-at timestamp.

3.3 Startup prune runs on the main actor

Plugins/ClipboardHistory/Sources/ClipboardHistoryController.swift

Issue. Decryption is already on the background serial queue; only finishLoading's prune returns to the main actor for a full pass.

Fix. ClipboardRetentionPolicy.prune is pure, so run it inside the existing background load task and hand the main actor the pruned result.

4. Robustness and hygiene

4.1 Killed helper processes are never reaped

Plugins/ClipboardHistory/Sources/ClipboardPasteboardReaderProcess.swift

Issue. terminateSession calls kill(pid, SIGKILL) with no corresponding wait, so repeated timeouts accumulate zombies for the lifetime of the host process.

Fix. After the kill, call process.waitUntilExit() from a detached task.

4.2 Polling continues across system sleep

Plugins/ClipboardHistory/Sources/ClipboardHistoryController.swift

Issue. The 500 ms interval itself is reasonable — macOS has no clipboard-change notification, and polling must continue while the panel is closed or the feature stops working. Waking during sleep is the avoidable part.

Fix. Observe NSWorkspace.willSleepNotification and didWakeNotification; invalidate the timer on sleep, rebuild it through startMonitoringIfPossible() on wake, and run one immediate processPasteboardChange() to pick up changes made while asleep.

4.3 pluginPaletteSelectableRow missing from the compatibility inventory

scripts/tests/test_plugin_minimum_host_compatibility.py

Issue. Every new top-level PluginKit type is registered at 1.3.0; the omission is this View extension method. PluginPaletteComponents.swift also has no inventory-coverage test equivalent to the component-theme one.

Fix. Add the method to NEW_API_MINIMUM_HOSTS and add a palette-component inventory test, so the next component added here cannot slip through the same way.

4.4 One-line secure-input guard

Plugins/ClipboardHistory/Sources/ClipboardSnippetKeywordExpander.swift

Issue. Secure classification relies solely on the AXSecureTextField subrole. To be precise about the actual exposure: WebKit and Chromium both map password inputs to that subrole, so browsers and Electron apps are already covered correctly. The residual gap is terminal password entry (Terminal's Secure Keyboard Entry is off by default and it reports AXTextArea) and a few custom-drawn controls, and it only applies when keystrokes match a configured keyword prefix.

Fix. Check IsSecureEventInputEnabled() before classifyEditor and return .secure when true. The benefit is bounded — the system already withholds key content from event taps while secure event input is active — but the cost is one line.

4.5 ClipboardHistoryPanel.swift is 7,291 lines

Issue. Not a functional problem, but a real cost to incremental compilation and to future review.

Fix. Split along the existing seams: search/result model, row views, preview, Actions panel, keyboard handling.

Suggested order

1.1 first — smallest diff, widest blast radius. Then 2.1, since it concerns user data being silently discarded. Then 3.1 through 3.3 as one change sharing the counters and timestamp. The rest can follow.

One process note: the PluginKit additions and the unified-palette extraction are independently reviewable and affect every plugin, but here they sit under 64k lines of plugin code — which is how 1.2 and 1.3 became easy to miss. Landing shared-framework work separately would make both halves easier to review. The WindowLayouts and CloudflareR2 edits are also unrelated drive-bys (a Swift 6.3 sendability workaround and a flaky-test timing fix); both look fine, they just belong elsewhere.

Thanks for the detailed review. I rechecked every finding against the current PR head and addressed the issues that should be fixed in this PR.

Fixed

• CI / Quick Look sendability: cancellation now owns an immutable request handle instead of capturing a non-Sendable Quick Look request in a @Sendable closure.
• Selected-row contrast: shared palette rows now use AppKit's semantic selected-text color, including high-contrast appearances, instead of hard-coded white.
• Return-key selector behavior: insertNewlineIgnoringFieldEditor: now selects the configured alternate action directly, while ordinary insertNewline: still uses the event modifiers. A regression test covers the AppKit selector path.
• Clipboard summary cost: history count, saved count, payload bytes, and the oldest unprotected item are cached and updated when the collection changes instead of being rescanned from panel-state getters.
• Capture retention cost: captures use an O(1) safety check and skip the full retention pass when no item could be expired or evicted. The full pass remains the fallback whenever capacity, expiration, or an in-flight durable mutation makes it necessary.
• Startup responsiveness: initial retention pruning now runs on the persistence worker. A settings-revision check reapplies changed retention settings before the result is published.
• Secure input: snippet expansion now checks macOS Secure Event Input before Accessibility classification and clears its buffer without inspecting the focused field. This has a regression test.
• PluginKit compatibility inventory: pluginPaletteSelectableRow is now inventoried at its introduction version, and the inventory parser/test covers public View extension APIs.

Kept intentionally

• Shortcut assignment while temporarily unavailable: canAssign remains based on whether the action is an interactive command, not its momentary availability. This lets users configure a shortcut even when the queue or current selection is not active; execution still reports the real unavailable state. The code now documents this distinction, and the existing regression test verifies it.
• Helper-process reaping: the helper uses Foundation.Process, which reaps the child when it terminates. I also ran an isolated 25-process forced-termination probe and observed no remaining processes or zombies, so adding another wait/reaping mechanism would duplicate Foundation's ownership.
• Queue HUD fallback timer: this is a main-run-loop timer, so it is suspended during system sleep and does not wake the Mac. It only resumes the lightweight fallback check after wake, within at most 0.5 seconds.

Follow-up rather than a partial fix here

• Whole-database rollback/replacement detection: this is a valid hardening topic, but a manifest stored beside or inside the same database cannot detect replacement with an older, internally valid database-plus-manifest pair. A meaningful solution needs an external Keychain-anchored store identity or monotonic generation plus an explicit backup/restore policy. That should be designed separately so legitimate restores are not mistaken for tampering.
• Splitting the large clipboard panel file: worthwhile for maintenance, but it is a structural refactor with no user-visible correction and would add review risk to an already large feature PR.

Validation on the updated head includes the full local make ci suite after merging current main, the Clipboard History plugin build after the final capture-path cleanup, and the successful current-head GitHub Actions Build.